- 
                Notifications
    You must be signed in to change notification settings 
- Fork 421
Support constructing blinded path onion keys #2411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support constructing blinded path onion keys #2411
Conversation
15ba6d4    to
    769e222      
    Compare
  
    | Codecov ReportPatch coverage:  
 
 ❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@            Coverage Diff             @@
##             main    #2411      +/-   ##
==========================================
+ Coverage   90.35%   91.02%   +0.67%     
==========================================
  Files         106      106              
  Lines       56242    62890    +6648     
  Branches    56242    62890    +6648     
==========================================
+ Hits        50816    57246    +6430     
- Misses       5426     5644     +218     
 ☔ View full report in Codecov by Sentry. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good!
769e222    to
    87a64bc      
    Compare
  
    | CI failure is #2385. | 
d230f57    to
    b694acd      
    Compare
  
    | Rebased to fix CI. | 
        
          
                lightning/src/ln/onion_utils.rs
              
                Outdated
          
        
      | let route_hop = match route_hop_opt { | ||
| Some(hop) => hop, | ||
| None => { | ||
| res = Some((None, None, true)); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all these lines where we set res = Some((None, None, true)); when the error is from a blinded hop, do we set them to be retryable just because we don't have a way of knowing otherwise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, if we return payment_retryable = false here then we mark the payment as abandoned in outbound_payment, which would be bad since it would prevent further payment retries. A payment is only not retryable if the recipient explicitly rejects it, which we have no way of knowing with blinded path errors.
Get rid of a bunch of indentation and be more idiomatic.
And fix an its vs it's grammar
b694acd    to
    4af6d98      
    Compare
  
    4af6d98    to
    80a949d      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering was there any reason the "Add missing test coverage for bogus err packet with valid hmac" and "Struct-ify decoded onion failures" commits got dropped?
80a949d    to
    d858987      
    Compare
  
    | 
 I was planning to split them into a different PR with another onion failure decode refactor, but ended up tabling that refactor for now. So, I re-added them here, sorry for the confusion lol. | 
We don't bother actually parsing errors from within a blinded path, since all errors should be wiped by the introduction node by the time it gets back to us (the sender).
Useful for generating a next hop blinding point when forwarding a blinded payment.
To avoid several long hard-to-read tuple return values.
d858987    to
    7b31712      
    Compare
  
    
Lays some groundwork for route blinding support, helping address #1970.
Prior to this PR we would only construct onion keys for unblinded path hops, and also only parse onion failures from unblinded path hops. This is tested in #2413.